-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New Smart integer scaling implementation to address #18154 #18296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Smart integer scaling implementation to address #18154 #18296
Conversation
|
Hi, I'll ask for feedback on this one before merging. |
|
No worries, I messaged sonninnos on Discord the same day I opened this PR and they said they'd look at this soon. Wasn't expecting this to get merged yet. |
|
Finally did some testing, and otherwise it seems to work nicely as-is, except I'd just get rid of the fallback to non-integer completely, since otherwise the hardcoded 0.88 really needs to be adjustable, because currently it for example ruins DOS 640x400 at 1080p with half steps. I feel it is easy enough to rather manually disable integer scaling if it does not work well enough for the current situation. Or why not a new separate "smart+fallback" option. |
|
I'm willing to make this configurable, I think that'd be the ideal solution. The only reason I hard-coded the numbers was that I've never tried changing RetroArch before, let alone add new configs. So I didn't want to spend a lot of time figuring out how to make it configurable before I knew if my proposal would be rejected. Since I rewrote pretty much all of the smart scaling code without consulting anyone I was worried there'd be some pushback.
This is true if you only ever use one display, which I'm sure is a very common approach; you can just set some overrides and forget about it. But I couldn't find a good solution if you switch between multiple displays, which is especially relevant for laptops and handhelds like Nintendo Switch or Steam Deck. I'd really like a solution that lets the user describe their scaling preferences so it'll still do the right thing if the resolution changes. I guess the main question now is if we want to keep having special treatment 240p and 480i/p or not. I can see arguments in both directions; if you can already configure the maximum % of content that gets cropped, you can handle overscan on a core-by-core basis that way. On the other hand SDTV resolutions are a very common use case and cores can apply varying amounts of overscan cropping so it might still be handy to be able to be able to say "use overscan if less than 15% of the 240p content gets cropped" regardless of whether the core's content height is 240, 224 or 216 for example. |
|
I can merge this and add the adjustment option with a follow-up PR once I get back to the coding zone. |
|
Okeydoke, just realized my mentioned problem is of course sorted by moving the non-integer fallback after the half-scales are actually calculated. So let's see if we can manage without adding the value option or separate smart-mode. So this should cover it: 933baca |
Description
This pull request proposes a different approach to smart scaling. It chooses overscaling for 240p and 480p content when it would result in at least 192 or 384 lines being shown (i.e. the title safe area); other content (mainly handhelds) is allowed up to 6 cropped lines to accommodate a few resolutions which almost match the overscaled content. Otherwise, underscaling is chosen, as long as it doesn't result in a margin larger than 12% (a threshold I chose by testing various content/display combinations.) If neither condition is satisfied, non-integer scaling is used. In other words, it only opts for integer scaling when it's "close enough."
I've tried to follow the Coding Standards as best I could, but this is my first contribution to RetroArch and I don't typically code in C, so I apologize in advance for any missteps.
Related Issues
"Smart" integer scaling is bugged #18154
In short, this change is trying to address the fact that the current Smart scaling implementation 1) doesn't behave consistently with regards to choosing underscaling or overscaling, and 2) sometimes allows excessive amounts of cropping or letterboxing. My guiding principle was to choose heuristics that would be simple for the end user to understand and unlikely to annoy.
Examples
224p scaled to 1280x720
Underscaled with 6.67% margin.

224p scaled to 1280x800 (Steam Deck)
Overscaled with 16.67% cropping

224p scaled to 1920x1080
Overscaled with 10% cropping.

480p scaled to 1280x720
Stretched to fit. Overscaling would crop 25%, underscaling would waste 1/3 of the screen's height.

480p scaled to 1920x1080
Underscaled with 11.11% margin.

GBA (160p) scaled to 1280x720
Underscaled with 11.11% margin.

GBA (160p) scaled to 1920x1080
Overscaled with 3 lines cropped from the top and bottom each.

PSP (272p) scaled to 1280x720
Stretched to fit. Overscaling would crop 16 lines from the top and bottom each, underscaling would waste 24.4% of the screen height.

PSP (272p) scaled to 1280x800 (Steam Deck)
Also stretched to fit, but this example demonstrates that the scaling works correctly when the screen aspect ratio is taller than the content.

PSP (272p) scaled to 1920x1080
Overscaled with

2 lines1 line cropped from the top and bottom each.Future work
Ideally the cutoff for choosing between underscaling and stretching would be configurable, but I have no experience implementing new config settings.
Reviewers
@sonninnos According to Git blame, they wrote the initial Smart scaling implementation.